-
Notifications
You must be signed in to change notification settings - Fork 21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
style: adjust the style of the floating icon. #269
Conversation
Caution Review failedThe pull request is closed. 概述浏览此次更改主要涉及浮动按钮和提供者私有类的功能调整。在浮动按钮组件中,重命名了 变更
序列图sequenceDiagram
participant App
participant ProviderPrivate
participant FloatingButton
App->>ProviderPrivate: onNotifyFloatingIconChanged
ProviderPrivate-->>App: 注册回调
FloatingButton->>ProviderPrivate: 触发浮动图标变更事件
ProviderPrivate->>App: 执行回调通知
这个序列图展示了浮动图标变更事件的处理流程,包括注册回调、事件触发和通知机制。 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (33)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
packages/providers/inpage-providers-hub/src/floatingButton/index.tsx
(3 hunks)packages/providers/onekey-private-provider/src/ProviderPrivate.ts
(2 hunks)
🔇 Additional comments (3)
packages/providers/onekey-private-provider/src/ProviderPrivate.ts (2)
48-49
: 新增的事件类型命名合理且清晰
事件命名遵循了现有的命名约定,保持了一致性。
90-95
: 事件处理逻辑简洁有效
事件处理的实现方式简单直接,易于理解和维护。
packages/providers/inpage-providers-hub/src/floatingButton/index.tsx (1)
673-673
: 浮动图标位置调整合理
将图标位置从顶部调整到底部25%的位置,提升了用户体验。
packages/providers/inpage-providers-hub/src/floatingButton/index.tsx
Outdated
Show resolved
Hide resolved
packages/providers/inpage-providers-hub/src/floatingButton/index.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
packages/providers/inpage-providers-hub/src/floatingButton/index.tsx
(5 hunks)
🔇 Additional comments (2)
packages/providers/inpage-providers-hub/src/floatingButton/index.tsx (2)
505-506
: 当 dapp.name 为空时,会回退到 host。请确认该逻辑是否符合需求。
763-763
: 与之前评论相同:请移除 console.log,以免在生产环境留下多余日志。
…rovider into floating-icon-ui
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
packages/core/src/consts.ts
(1 hunks)packages/extension/extension-bridge-hosted/src/JsBridgeExtBackground.ts
(2 hunks)packages/providers/onekey-private-provider/src/ProviderPrivate.ts
(2 hunks)
🔇 Additional comments (4)
packages/core/src/consts.ts (1)
14-14
: 代码变更正确!
常量命名规范统一,值的格式保持一致。
packages/providers/onekey-private-provider/src/ProviderPrivate.ts (2)
53-53
: 需要添加类型定义
建议为 _listeners
数组添加更严格的类型定义。
- _listeners: { type: string, callback: (params: any) => void }[] = []
+ type ListenerCallback = (params: { showFloatingIcon: boolean }) => void;
+ _listeners: { type: string, callback: ListenerCallback }[] = []
104-106
: 需要添加事件监听器清理方法
为防止内存泄漏,建议添加移除监听器的方法。
+ removeNotifyFloatingIconChanged(callback: (params: {showFloatingIcon: boolean}) => void) {
+ this._listeners = this._listeners.filter(listener =>
+ listener.type !== METHODS.wallet_events_floating_icon_changed ||
+ listener.callback !== callback
+ );
+ }
packages/extension/extension-bridge-hosted/src/JsBridgeExtBackground.ts (1)
14-14
: 常量更新正确实施
常量重命名和使用更新都已正确完成,代码功能保持不变。
Also applies to: 146-146
packages/providers/onekey-private-provider/src/ProviderPrivate.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
packages/providers/inpage-providers-hub/src/floatingButton/index.tsx (1)
Line range hint
726-777
: 注入逻辑需要错误处理建议添加错误处理机制:
async function injectIcon() { + try { const { isShow, i18n: i18nResponse } = await (globalThis as unknown as { $onekey: { $private: { request: ( arg: { method: string; params: { url: string } } ) => Promise<{ isShow: boolean, i18n: i18nText }> } } }).$onekey.$private.request({ method: 'wallet_isShowFloatingButton', params: { url: window.location.origin }, }); // ... 其余代码 ... + } catch (error) { + console.error('注入浮动图标失败:', error); + return false; + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
packages/providers/inpage-providers-hub/src/floatingButton/index.tsx
(8 hunks)
🔇 Additional comments (4)
packages/providers/inpage-providers-hub/src/floatingButton/index.tsx (4)
46-48
: 函数重命名提高了代码清晰度
函数名从 removeApp
改为 removeIcon
更准确地描述了其功能。
519-521
: 交互体验改进
添加了鼠标指针样式和对齐方式,提升了用户体验。
760-769
: 🛠️ Refactor suggestion
事件监听器需要清理机制
建议在组件卸载时移除事件监听器,防止内存泄漏。
export function injectFloatingButton() {
+ const cleanup = () => {
+ removeIcon();
+ // 清理事件监听器
+ };
+
+ if (typeof window !== 'undefined') {
+ window.addEventListener('unload', cleanup);
+ }
// ... 其余代码 ...
}
Likely invalid or redundant comment.
685-685
: 浮动按钮位置调整
将按钮位置从顶部调整到底部 25% 处,建议验证在不同屏幕尺寸下的表现。
@@ -461,6 +462,7 @@ function SecurityInfo({ | |||
color: 'rgba(0, 0, 0, 0.88)', | |||
fontSize: '13px', | |||
fontWeight: '500', | |||
overflow: 'hidden', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
文本溢出处理优化
添加了文本溢出处理,防止长文本破坏布局。建议进一步完善:
style={{
width: '100%',
overflow: 'hidden',
textOverflow: 'ellipsis'
+ whiteSpace: 'nowrap'
}}
Also applies to: 505-513
03ecdab
to
4b4f034
Compare
Summary by CodeRabbit